New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Precise RPCTime in muons #18681
Precise RPCTime in muons #18681
Conversation
A new Pull Request was created by @jhgoh (Junghwan John Goh) for master. It involves the following packages: RecoMuon/MuonIdentification @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
In this PR the RPCTime is filled with the precise timing information if not If I read correctly, timeError is initialized to 0 in the RPCRecHit constructor (DataFormats/RPCRecHit/src/RPCRecHit.cc) and it is only modified if timeError>=0: https://cmssdt.cern.ch/lxr/source/RecoLocalMuon/RPCRecHit/src/RPCRecHitBaseAlgo.cc#0043 If so, the bx should never be used here, not even for rpc in phase0/1: could you please check? (If my observation is correct, I would expect this PR may affect also phase0/1 reco, and not only phase 2. On the other hand, I can't see any modification in jenkins outputs, not even for RPC in phase 2: could you please suggest me which distribution I should look at, and possibly also provide yourself here some evidence that this update behaves as you want, and does not affect current reco)? |
On 5/15/17 3:58 AM, perrotta wrote:
In this PR the RPCTime is filled with the precise timing information if not
hitRPC->timeError() < 0
If I read correctly, timeError is initialized to 0 in the RPCRecHit
constructor (DataFormats/RPCRecHit/src/RPCRecHit.cc) and it is only
modified if timeError>=0:
https://cmssdt.cern.ch/lxr/source/RecoLocalMuon/RPCRecHit/src/RPCRecHitBaseAlgo.cc#0043
If so, the bx should never be used here, not even for rpc in phase0/1:
could you please check?
(If my observation is correct, I would expect this PR may affect also
phase0/1 reco, and not only phase 2. On the other hand, I can't see any
modification in jenkins outputs, not even for RPC in phase 2:
what about this
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_9_2_X_2017-05-09-2300+18681/19931/validateJR/all_OldVSNew_RunPhoton2012Bwf4p53/
?
… could you
please suggest me which distribution I should look at, and possibly also
provide yourself here some evidence that this update behaves as you
want, and does not affect current reco)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18681 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbhMs7qku-j6KXia3AYUHEDc-JY_8ks5r6C_BgaJpZM4NXLCA>.
|
Pull request #18681 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
@perrotta Thank you for checking details. I missed the point. -> Use bx based time if timeError < 0 |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Your latest commit is also going to affect (fix, I'd say...) validations: In fact, that line was merged with #18632 five days ago, so probably nobody still checked its outcome. It would be interesting to see how the fix implemented here will act on RPC time plots (I expect that you get now some result different from 0 also for phase1/0) |
The timing of RPCRecHits are new variables from 92X (91X backport is not merged yet). So, we did not have a chance to make comparisons yet. What do I expect with this change in Phase1/0?
|
Comparison is ready Comparison Summary:
|
With the latest fix, now RPC time only gets modified for Phase 2, e.g. (TTbar14TeV2023D4): Corresponding entries remain unchanged in Phase1 RPC, as it should. As expected, the DQM plots for RPC are modified also in Phase 1, but this is simply due to the bug fix applied here which avoids having all rpcTIme=0 at phase 1 (as it was after #18632, see above #18681 (comment). Example for 10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017: Thus everything goes as expected, now. I remain with only one doubt: does hitRPC->time() refer to an "absolute time" computed with respect to BX=0, or does it refer to the time diff with respect to hitRPC->BunchX() instead? @jhgoh : could you please verify and let us know? |
@perrotta thank you for comments and providing comparison plots. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
+1 |
The RPCTime information is computed based on the bunch crossing x 25ns, not taking the precise timing information which is available from the phase-II upgrade.
With this PR, the RPCTime in the reco::Muon is computed using the precise time measurement if available.
@ptraczyk